Returns real remote and local address instead mocked #299
Conversation
|
I would like to get this in if possible. It's the only thing this library lacks from gorilla/websocket. |
netconn_notjs.go
Outdated
| } | ||
|
|
||
| func (c *netConn) RemoteAddr() net.Addr { | ||
| if ra, ok := c.c.rwc.(LocalRemoteAddr); ok { |
There was a problem hiding this comment.
Not sure how this could work? Have you tested this? c.c.rwc is not a net.Conn and so doesn't have these methods you're checking for.
There was a problem hiding this comment.
It only works in tests because the tests don't use a real http transport/server, they're using my pipe abstraction. See https://github.com/nhooyr/websocket/blob/master/internal/test/wstest/pipe.go
There was a problem hiding this comment.
It works fine. Its check the reader/writer if it also has RemoteAddr() and LocalAddr() defined. It is not checking if it net.conn.
There was a problem hiding this comment.
@nhooyr Here is example of something similar to what I did:
https://github.com/golang/go/blob/master/src/io/io.go#L408
There was a problem hiding this comment.
Have you tested this in production? There's no RemoteAddr or LocalAddr on the bufio.ReadWriter returned to us from the transport.
There was a problem hiding this comment.
This the echo example.
There was a problem hiding this comment.
Ok now I'm super confused I'll have to look into how that's possible lol
|
Going to close as I'm assuming this approach won't work. Leaving the issue open at #327 though so I can figure something out here. |
|
Ah I understand, it works server side but would not work with connections from |
|
What is being store with io.ReadWriterClose when using the websocket.Dial? io.ReadWriterClose is just common abstraction. If the io.ReadWriterClose is being used with net.Conn then we know the local address and remote address methods exists. Just because we see io.ReadWriterClose when looking at the code does not hide the ability to access other methods if what being stored there during runtime supports these methods. Can you provide any screenshots of what is being stored there? |
|
I don't fully remember the type but it's not a net.Conn. Grep the net/http.Transport code for protocol upgrade and you'll find it. I'm heading out right now otherwise I'd link you. |
|
I will do exploring with that when I get some time. Thanks for your responses. |
|
We don't get a |
|
Thanks @photostorm |
Currently, RemoteAddr() and LocalAddr() are mocked. This change allow the real RemoteAddr() and LocalAddr() to be used.